refactor(atomic): Migrate SmartSnippet functional components #6602

Copilot · 2025-11-20T13:48:36Z

Migrates 5 SmartSnippet functional components from Stencil to Lit framework, creating separate files for each component with dedicated test files.

Description

Successfully migrated all SmartSnippet functional components to Lit:

SmartSnippetWrapper - Wrapper with heading and article
SmartSnippetQuestion - Question heading
SmartSnippetTruncatedAnswer - Truncated answer content
SmartSnippetFooter - Footer section
SmartSnippetFeedbackBanner - Feedback UI with like/dislike buttons

Each component has been separated into its own file with a dedicated test file, following established Lit patterns and using proper test utilities (renderFunctionFixture, createTestI18n) for consistent test structure and automatic cleanup.

Changes Made

New Lit Functional Components:

snippet-wrapper.ts - Wrapper component with 8 unit tests
snippet-question.ts - Question component with 7 unit tests
snippet-truncated-answer.ts - Truncated answer component with 6 unit tests
snippet-footer.ts - Footer component with 5 unit tests
smart-snippet-feedback-banner.ts - Feedback banner component with 21 unit tests (using multiClassMap for improved maintainability)

Renamed Stencil Files (for backward compatibility):

stencil-smart-snippet-common.tsx - Legacy Stencil versions
stencil-smart-snippet-feedback-banner.tsx - Legacy Stencil version

Updated Imports:

atomic-smart-snippet.tsx - Updated to reference renamed Stencil versions
atomic-insight-smart-snippet.tsx - Updated to reference renamed Stencil versions

Testing

✅ 47 comprehensive unit tests covering all functional components
✅ Tests use renderFunctionFixture for proper cleanup and consistent patterns
✅ Tests validate props, rendering, conditional logic, event handlers, i18n integration, and children rendering
✅ Linting passing
✅ All functionality preserved from Stencil versions

Code Quality Improvements

✅ Each component in its own file for better organization
✅ Dedicated test file per component
✅ Used multiClassMap directive for conditional classes (improved maintainability)
✅ Proper test utilities (renderFunctionFixture, createTestI18n) for automatic cleanup

Migration Notes

The Lit versions are ready for use when the consuming Stencil components (atomic-smart-snippet and atomic-insight-smart-snippet) are migrated to Lit. The legacy Stencil versions remain available with the stencil- prefix to maintain backward compatibility until those migrations are complete.

Fixes Migrate SmartSnippet functional components #6351

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Migrate 5 SmartSnippet functional components from Stencil to Lit: - SmartSnippetWrapper - SmartSnippetQuestion - SmartSnippetTruncatedAnswer - SmartSnippetFooter - SmartSnippetFeedbackBanner Changes: - Created new Lit versions of all functional components - Renamed original Stencil files with `stencil-` prefix - Updated imports in components using these functions - Added comprehensive unit tests for all migrated components - All tests passing and linting clean Co-authored-by: alexprudhomme <[email protected]>

...components/common/smart-snippets/atomic-smart-snippet/smart-snippet-functional-components.ts

alexprudhomme · 2025-11-20T15:15:51Z

@copilot retry this please !

Co-authored-by: alexprudhomme <[email protected]>

This reverts commit 56fa228.

Co-Authored-By: alexprudhomme <[email protected]>

Copilot

Pull Request Overview

This PR migrates SmartSnippet functional components from Stencil to Lit, establishing a clear migration path by creating new Lit implementations while preserving the legacy Stencil versions with deprecation markers.

Key Changes:

Created Lit-based functional components following established patterns (render* naming, FunctionalComponent types)
Renamed legacy Stencil components with stencil- prefix and added deprecation markers
Added comprehensive test coverage for all new Lit components
Updated imports in consuming Stencil components to reference the renamed legacy versions

Reviewed Changes

Copilot reviewed 12 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`atomic-smart-snippet.tsx`	Updated imports to use `stencil-` prefixed legacy components
`atomic-insight-smart-snippet.tsx`	Updated imports to use `stencil-` prefixed legacy components
`stencil-smart-snippet-feedback-banner.tsx`	Renamed legacy Stencil feedback banner with deprecation marker
`smart-snippet-feedback-banner.ts`	New Lit implementation of feedback banner functional component
`smart-snippet-feedback-banner.spec.ts`	Comprehensive test coverage for Lit feedback banner (251 test cases)
`stencil-smart-snippet-common.tsx`	Renamed legacy Stencil common components with deprecation markers
`snippet-wrapper.ts`	New Lit wrapper component for smart snippets
`snippet-wrapper.spec.ts`	Test coverage for wrapper component
`snippet-question.ts`	New Lit question heading component
`snippet-question.spec.ts`	Test coverage for question component
`snippet-truncated-answer.ts`	New Lit truncated answer component
`snippet-truncated-answer.spec.ts`	Test coverage for truncated answer component
`snippet-footer.ts`	New Lit footer component
`snippet-footer.spec.ts`	Test coverage for footer component

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-20T18:39:37Z

packages/atomic/src/components/common/smart-snippets/smart-snippet-feedback-banner.spec.ts

+      }),
+    } as unknown as i18n;
+  });
+


The container element is appended to document.body in beforeEach but never removed in an afterEach. This could cause memory leaks and test pollution. Add cleanup:

afterEach(() => { document.body.removeChild(container); });

Suggested change

afterEach(() => {

if (container && container.parentNode === document.body) {

document.body.removeChild(container);

}

});

Fair catch. @alexprudhomme if you don't use the render fixture, you have to own the clean up ;)

Copilot · 2025-11-20T18:39:37Z

packages/atomic/src/components/common/smart-snippets/smart-snippet-feedback-banner.ts

+        class=${
+          props.liked
+            ? 'flex items-center gap-x-1.5 text-success'
+            : 'flex items-center gap-x-1.5 cursor-pointer hover:underline'
+        }


[nitpick] The common classes 'flex items-center gap-x-1.5' are duplicated in both branches of the ternary. Consider using the classMap directive or separating common classes to improve maintainability:

class="flex items-center gap-x-1.5 ${props.liked ? 'text-success' : 'cursor-pointer hover:underline'}"

Or using classMap:

class=${classMap({ 'flex': true, 'items-center': true, 'gap-x-1.5': true, 'text-success': props.liked, 'cursor-pointer': !props.liked, 'hover:underline': !props.liked, })}

The same applies to the dislike button classes on lines 58-62.

jmazzah-coveo

why are there new components with this migration? i don't see them used anywhere. like snippet-footer, snippet-question, etc? will we need them in the future?

alexprudhomme · 2025-11-24T18:24:04Z

packages/atomic/src/components/common/smart-snippets/smart-snippet-feedback-banner.spec.ts

+  let container: HTMLElement;
+  let mockI18n: i18n;
+
+  beforeEach(() => {


@copilot Use the renderFunctionFixture instead. Look at the other file and do something similar.

Updated in commit e20e9ec. The test now uses renderFunctionFixture and createTestI18n following the same pattern as the other separated component test files.

…-banner tests Updated test file to use renderFunctionFixture and createTestI18n helpers for proper cleanup and consistent test patterns. This addresses the review comment about using the renderFunctionFixture pattern. Co-authored-by: alexprudhomme <[email protected]>

Copilot · 2025-11-24T18:31:59Z

why are there new components with this migration? i don't see them used anywhere. like snippet-footer, snippet-question, etc? will we need them in the future?

These are functional components currently used by the Stencil smart-snippet components (atomic-smart-snippet and atomic-insight-smart-snippet). The migration creates Lit versions of these functions while keeping the Stencil versions (with stencil- prefix) so the Stencil components can continue using them. When those Stencil components are eventually migrated to Lit, they'll use these new Lit functional components, and then the stencil- prefixed versions can be removed.

…nner Improved maintainability by using multiClassMap directive instead of ternary operators for conditional classes. This separates common classes from conditional ones and makes the code more readable. Co-authored-by: alexprudhomme <[email protected]>

Initial plan

fc53c65

Copilot AI assigned Copilot and alexprudhomme Nov 20, 2025

Copilot started work on behalf of alexprudhomme November 20, 2025 13:49 View session

Copilot AI requested a review from alexprudhomme November 20, 2025 14:11

alexprudhomme reviewed Nov 20, 2025

View reviewed changes

...components/common/smart-snippets/atomic-smart-snippet/smart-snippet-functional-components.ts Outdated Show resolved Hide resolved

Copilot started work on behalf of alexprudhomme November 20, 2025 15:00 View session

Copilot AI requested a review from alexprudhomme November 20, 2025 15:14

Copilot started work on behalf of alexprudhomme November 20, 2025 15:17 View session

Changes before error encountered

56fa228

Co-authored-by: alexprudhomme <[email protected]>

Revert "Changes before error encountered"

5fb331d

This reverts commit 56fa228.

alexprudhomme temporarily deployed to PR Artifacts November 20, 2025 18:21 — with GitHub Actions Inactive

cool

b4b0a79

Co-Authored-By: alexprudhomme <[email protected]>

alexprudhomme temporarily deployed to PR Artifacts November 20, 2025 18:25 — with GitHub Actions Inactive

separate

821af93

Co-Authored-By: alexprudhomme <[email protected]>

alexprudhomme temporarily deployed to PR Artifacts November 20, 2025 18:29 — with GitHub Actions Inactive

alexprudhomme changed the title ~~[WIP] Migrate SmartSnippet functional components~~ refactor(atomic): Migrate SmartSnippet functional components Nov 20, 2025

alexprudhomme marked this pull request as ready for review November 20, 2025 18:34

alexprudhomme requested a review from a team as a code owner November 20, 2025 18:34

alexprudhomme requested review from Copilot, fbeaudoincoveo and louis-bompart November 20, 2025 18:34

Copilot started reviewing on behalf of alexprudhomme November 20, 2025 18:34 View session

alexprudhomme temporarily deployed to Prerelease (CDN) November 20, 2025 18:35 — with GitHub Actions Inactive

Copilot finished reviewing on behalf of alexprudhomme November 20, 2025 18:38

Copilot AI reviewed Nov 20, 2025

View reviewed changes

jmazzah-coveo approved these changes Nov 24, 2025

View reviewed changes

alexprudhomme reviewed Nov 24, 2025

View reviewed changes

Copilot started work on behalf of alexprudhomme November 24, 2025 18:24 View session

Copilot AI requested a review from alexprudhomme November 24, 2025 18:35

Copilot finished work on behalf of alexprudhomme November 24, 2025 18:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(atomic): Migrate SmartSnippet functional components #6602

refactor(atomic): Migrate SmartSnippet functional components #6602

Copilot AI commented Nov 20, 2025 •

edited

Loading

Uh oh!

Uh oh!

alexprudhomme commented Nov 20, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 20, 2025

Uh oh!

louis-bompart Nov 24, 2025

Uh oh!

Copilot AI Nov 20, 2025

Uh oh!

jmazzah-coveo left a comment

Uh oh!

alexprudhomme Nov 24, 2025

Uh oh!

Copilot AI Nov 24, 2025

Uh oh!

Copilot AI commented Nov 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

+  afterEach(() => {
+    if (container && container.parentNode === document.body) {
+      document.body.removeChild(container);
+    }
+  });

refactor(atomic): Migrate SmartSnippet functional components #6602

Are you sure you want to change the base?

refactor(atomic): Migrate SmartSnippet functional components #6602

Conversation

Copilot AI commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes Made

Testing

Code Quality Improvements

Migration Notes

Uh oh!

Uh oh!

alexprudhomme commented Nov 20, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes:

Reviewed Changes

Uh oh!

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

louis-bompart Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

jmazzah-coveo left a comment

Choose a reason for hiding this comment

Uh oh!

alexprudhomme Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Nov 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Copilot AI commented Nov 20, 2025 •

edited

Loading